Skip to content

Python 3.7+ compatibility#2

Open
mm wants to merge 3 commits intomasterfrom
py37-compat
Open

Python 3.7+ compatibility#2
mm wants to merge 3 commits intomasterfrom
py37-compat

Conversation

@mm
Copy link
Copy Markdown

@mm mm commented Dec 19, 2023

✨ PR Description

Purpose: Update Django Piston codebase to ensure compatibility with Python 3.7+ by modernizing deprecated Django imports and string handling patterns.

Main changes:

  • Replaced deprecated django.core.urlresolvers with django.urls imports across authentication, documentation, emitters and utilities modules
  • Updated render_to_response calls to use django.shortcuts.render function with explicit request context parameter
  • Modified ForeignKey definitions to include required on_delete=models.CASCADE parameter for Django 2.0+ compatibility

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how

@mm mm added the holding label Dec 19, 2023
@linearb
Copy link
Copy Markdown

linearb Bot commented Jan 23, 2026

✨ PR Review

The PR attempts to modernize the codebase for Python 3.7+ compatibility, but contains critical Python 2 syntax that will break in Python 3, including incompatible string decoding and invalid import statements.

3 issues detected:

🐞 Bug - Using Python 2's string decode("base64") method which doesn't exist in Python 3 🛠️

Details: The decode("base64") method does not exist in Python 3 and will cause an AttributeError at runtime. In Python 3, base64 decoding must be done using the base64 module with base64.b64decode().
File: piston/authentication.py (60-60)
🛠️ A suggested code correction is included in the review comments.

🐞 Bug - Attempting to import urlparse as a standalone module, which doesn't exist in Python 3 🛠️

Details: Line 1 imports 'urlparse' as a top-level module, which does not exist in Python 3. In Python 3, urlparse functionality is available only through urllib.parse (which is correctly imported on line 3). This will cause an ImportError when the module is loaded.
File: piston/models.py (1-1)
🛠️ A suggested code correction is included in the review comments.

🐞 Bug - Passing fields as a positional argument instead of using the 'args' or 'kwargs' named parameter 🛠️

Details: Django's reverse() function expects 'args' or 'kwargs' as named parameters, not positional. Passing 'fields' as a second positional argument may not work correctly and could cause TypeErrors or incorrect URL generation depending on the Django version.
File: piston/emitters.py (287-287)
🛠️ A suggested code correction is included in the review comments.

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants